Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add package and source package batching for publish #1146

Merged
merged 1 commit into from
Sep 18, 2024

Conversation

hstct
Copy link
Contributor

@hstct hstct commented Sep 2, 2024

closes #1147

@hstct hstct force-pushed the add_batching_for_publish branch 10 times, most recently from 38e7a34 to 25a30c3 Compare September 4, 2024 07:38
@hstct hstct marked this pull request as ready for review September 4, 2024 08:39
@hstct hstct force-pushed the add_batching_for_publish branch 5 times, most recently from 225bd1c to c4f3fe5 Compare September 10, 2024 10:08
Copy link

@dosas dosas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to add some tests for this?

@hstct hstct force-pushed the add_batching_for_publish branch 5 times, most recently from 221f0be to 9b3cebd Compare September 11, 2024 13:46
@hstct hstct changed the title Add package and source package batching for structured publish Add package and source package batching for publish Sep 11, 2024
@hstct
Copy link
Contributor Author

hstct commented Sep 11, 2024

Would it be possible to add some tests for this?

Existing tests should cover all of this. These are all performance boosts.

@hstct hstct force-pushed the add_batching_for_publish branch 2 times, most recently from a035fbc to 17dede0 Compare September 11, 2024 13:57
@hstct hstct force-pushed the add_batching_for_publish branch 2 times, most recently from f41bc2b to fded6a2 Compare September 17, 2024 14:34
Copy link
Collaborator

@quba42 quba42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I tested appears to work without changes, but at more than twice the speed from before.

What I have below are small nitpicks, or questions about points that look the most risky to me. They don't necessarily require a change.

What I did not test: Source packages sync, upload, repeated upload (interesting because of the change to the retrieve method).

CHANGES/1147.feature Outdated Show resolved Hide resolved
pulp_deb/app/serializers/content_serializers.py Outdated Show resolved Hide resolved
pulp_deb/app/serializers/content_serializers.py Outdated Show resolved Hide resolved
pulp_deb/app/tasks/publishing.py Show resolved Hide resolved
pulp_deb/app/tasks/publishing.py Show resolved Hide resolved
@hstct
Copy link
Contributor Author

hstct commented Sep 18, 2024

Simplified the PR and removed some of the "optimization" changes that are unrelated to this PR and they don't affect performance noticably.

@quba42
Copy link
Collaborator

quba42 commented Sep 18, 2024

It all LGTM, the only remaining question is, if we need the batching.

@hstct hstct merged commit 581f9f3 into pulp:main Sep 18, 2024
12 checks passed
@hstct hstct deleted the add_batching_for_publish branch September 18, 2024 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create PublishedArtifacts in bulk in publications
3 participants